Skip to content

datetime: Set the default year to the year during compile#2133

Merged
JF002 merged 1 commit into
mainfrom
compile-time-year
Oct 27, 2024
Merged

datetime: Set the default year to the year during compile#2133
JF002 merged 1 commit into
mainfrom
compile-time-year

Conversation

@FintasticMan

Copy link
Copy Markdown
Member

Supersedes #2067.

@FintasticMan FintasticMan added enhancement Enhancement to an existing app/feature UI/UX User interface/User experience labels Oct 2, 2024
@FintasticMan FintasticMan added this to the 1.15.0 milestone Oct 2, 2024
@FintasticMan FintasticMan requested a review from a team October 2, 2024 22:12
@github-actions

github-actions Bot commented Oct 2, 2024

Copy link
Copy Markdown

Build size and comparison to main:

Section Size Difference
text 374544B 16B
data 948B 0B
bss 63488B 0B

@tituscmd

tituscmd commented Oct 2, 2024

Copy link
Copy Markdown
Contributor

While were at it changing stuff related to Date & Time, we could also consider this PR:

#1782

@NeroBurner NeroBurner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. When tested on device good to go

@mark9064 mark9064 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Haven't tested on HW

PS: Took me about 5 minutes to understand the const correctness of the arrays, but I think you've got the type right. Well done 😅

Comment thread src/components/datetime/DateTimeController.cpp Outdated
@FintasticMan

Copy link
Copy Markdown
Member Author

Yeah, const-correctness in C++ isn't very obvious haha. I'm not by my dev kit so I can't debug this, but it really doesn't seem to like calling SetTime in the constructor, as doing so seems to result in a non-booting firmware.

@FintasticMan FintasticMan marked this pull request as draft October 6, 2024 12:49
@mark9064

mark9064 commented Oct 6, 2024

Copy link
Copy Markdown
Member

Ah, that would be as system task doesn't exist yet so it hits a null ptr deref when it tries to push OnNewTime

Edit: I think just adding a systemtask null check should suffice?

@FintasticMan

Copy link
Copy Markdown
Member Author

Good thinking! Adding a NULL check should indeed fix it.

@FintasticMan

Copy link
Copy Markdown
Member Author

It boots now, but I haven't been able to get the time to not be read from .noinit 😅, so I can't test if it actually works. I'm sure if the battery runs out it'll reset the time (isn't it annoying having such good battery life!)

@NeroBurner

Copy link
Copy Markdown
Contributor

Is it possible to make a Firmware that always reads the noinit data I instead of the filesystem?

@mark9064

mark9064 commented Oct 7, 2024

Copy link
Copy Markdown
Member

image
So I tried @pipe01's InfiniEmu and it looks like we're off by a little bit. I'd guess the month and day are meant to be ones

@FintasticMan

Copy link
Copy Markdown
Member Author

That makes a lot of sense haha.

@FintasticMan

Copy link
Copy Markdown
Member Author

My watch just ran out of battery, so I can confirm: this does work on actual hardware!

@FintasticMan FintasticMan marked this pull request as ready for review October 9, 2024 23:50
Comment thread src/components/datetime/DateTimeController.cpp Outdated
@JF002 JF002 merged commit f1651c8 into main Oct 27, 2024
@FintasticMan FintasticMan deleted the compile-time-year branch October 27, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement to an existing app/feature UI/UX User interface/User experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants